Base native currency price for dApp Staking #1252
Conversation
Dinonard
requested review from
ashutoshvarma,
PierreOssun,
shaunxw and
ermalkaleci
| run_for_blocks(1); | ||
|
|
||
| assert!( | ||
| TierConfig::<Test>::get().number_of_slots > base_number_of_slots, |
There was a problem hiding this comment.
can we have assert for number of slots as well?
There was a problem hiding this comment.
You mean to use to calculate the expected number of slots and compare it with the one in the tier config? (if yes, no problem)
There was a problem hiding this comment.
I meant assert_eq!(TierConfig::<Test>::get().number_of_slots, 120)
There was a problem hiding this comment.
Ok, but the 120 should be derived from somewhere, right?
I've added the check below this one, but kept the old one to ensure change is in the right direction.
There was a problem hiding this comment.
I was thinking hardcoded number so you can visually verify change. number > base_number can be any number
There was a problem hiding this comment.
The formula for calculating number of slots is provided via an associated type, it's not dictated by the dAPp staking pallet itself.
This is why, as per your suggestion, I added this below the existing > check:
assert_eq!(
TierConfig::<Test>::get().number_of_slots,
<Test as Config>::TierSlots::number_of_slots(higher_price),
);Only it's not hardcoded but derived from the associated type function call.
| @@ -366,6 +366,7 @@ impl TierSlotsFunc for ShidenTierSlots { | |||
|
|
|||
| parameter_types! { | |||
| pub const MinimumStakingAmount: Balance = 50 * SDN; | |||
| pub const BaseNativeCurrencyPrice: FixedU128 = FixedU128::from_rational(5, 100); | |||
There was a problem hiding this comment.
Should it be maybe FixedU128::from_rational(5, 10); ?
There was a problem hiding this comment.
No, the baseline is 0.05$
EDIT: You're right, it should be adjusted to be larger for Shiden due to slot number formula change before.
Had a bug in my model spreadsheet, but 0.05$ is correct for Shiden. When calculation formula was changed, we also changed the base number of slots to 55 which matches the price of 5 cents.
Minimum allowed line rate is |
Pull Request Summary
Fixes an issue with threshold calculation in dApp staking.
The issue happens because the formula for tier threshold update did not account for threshold value saturation.
As
ASTRprice increased, so did the number of available slots, and as a result, thresholds saturated at the minimum possible value. Number of slots could keep increasing, while the thresholds remained the same.But as ASTR price dropped, this brought a negative change in the number of slots. Since number of slots was way
larger than the base number of slots, this seemed as a larger change that it was supposed to be, in respect to the current threshold.
This fix changes the way threshold is calculated so that the
new_number_of_slotsis always compared to thebase_number_of_slots. This way ensures thresholds are always updated in respect to what the baseline is.Note